Skip to content

Conversation

@AurelioDeRosa
Copy link
Member

Fixes gh-908

@dmethvin
Copy link
Member

Ah, so the encoded version works? Awesome!

@AurelioDeRosa
Copy link
Member Author

@agcolom did you have the chance to double-check this?

@agcolom
Copy link
Member

agcolom commented May 31, 2016

@AurelioDeRosa My local setup is no longer working for the core api (not sure what change broke it yet, and I cannot install anything as I get the error on npm install!) so I haven't been able to test this, sorry.

@kswedberg
Copy link
Member

@AurelioDeRosa this needs a corresponding <category> entry in the categories.xml file. Unfortunately, when I try to add <category name="Version 1.12&amp;2.2" slug="1.12&amp;2.2">…</category>, I get "Warning: Invalid slug: 1.12&2.2." Using a hyphen (-) works. Maybe we should use that instead?

@kswedberg
Copy link
Member

kswedberg commented Jun 16, 2016

@AurelioDeRosa ugh, I think I spoke too soon. While I don't get the error when I use <category name="Version 1.12-2.2" slug="1.12-2.2"> (with hyphens). The build process seems to ignore it. Maybe there's a stricter check when adding the version than when validating it?

@AurelioDeRosa
Copy link
Member Author

@kswedberg I managed to have the dash working. I think what was wrong is that $.uniqueSort() doesn't not include the category at the bottom of the file.

If everyone is happy, I can update the PR and merge it.

@kswedberg
Copy link
Member

Aha! You're correct, @AurelioDeRosa . That was the missing piece of the puzzle. Thanks so much for looking into this. Please update and merge.

@AurelioDeRosa
Copy link
Member Author

PR updated. I double checked and everything works correctly. As soon as I get approval, I'll merge it.

@mgol
Copy link
Member

mgol commented Jul 17, 2016

LGTM

@AurelioDeRosa AurelioDeRosa deleted the category-fix branch July 17, 2016 17:00
kswedberg pushed a commit to kswedberg/api.jquery.com that referenced this pull request Jul 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants